Fix order for JoinSwapShareAmountOut CLI args #3942
Conversation
github-actions
bot
added
the
C:x/gamm
ValarDragon
added
V:state/compatible/backport
proto/osmosis/gamm/v1beta1/tx.proto
Outdated
| string token_in_max_amount = 4 [ | ||
| (gogoproto.customtype) = "github.com/cosmos/cosmos-sdk/types.Int", | ||
| (gogoproto.moretags) = "yaml:\"share_out_amount\"", | ||
| (gogoproto.moretags) = "yaml:\"token_in_max_amount\"", | ||
| (gogoproto.nullable) = false | ||
| ]; | ||
| string token_in_max_amount = 5 [ | ||
| string share_out_amount = 5 [ | ||
| (gogoproto.customtype) = "github.com/cosmos/cosmos-sdk/types.Int", | ||
| (gogoproto.moretags) = "yaml:\"token_in_max_amount\"", | ||
| (gogoproto.moretags) = "yaml:\"share_out_amount\"", | ||
| (gogoproto.nullable) = false |
There was a problem hiding this comment.
No we can't edit the proto tx, should be editing the CLI code only.
|
The relevant code that should be edited is here: https://github.com/osmosis-labs/osmosis/blob/main/x/gamm/client/cli/tx.go#L137-L144 Osmocli doesn't have tooling right for now letting you change the arg order. Either we add that, or we make a custom parse fn (likely much easier) for this CLI tx. ( |
|
I think simply fixing the help text would be good enough. Also, I suspect other CLI commands related to x/gamm may have the same issue, so better give them a test as well. |
|
@ValarDragon thoughts on just changing the order of args on the usage text / README? |
|
Ah yeah, just changing the help text SGTM |
There was a problem hiding this comment.
TYSM! Really appreciate the issue close :)
If you get a chance, would you be down to add a changelog entry to breaking changes in the ## v13.1.2 section for
* [3647](https://github.com/osmosis-labs/osmosis/pull/3647), [3942](https://github.com/osmosis-labs/osmosis/pull/3942) (CLI) re-order the command line arguments for `osmosisd tx gamm join-swap-share-amount-out`
Added! |
|
Thank you! |
* fix order for JoinSwapShareAmountOut args * Revert "fix order for JoinSwapShareAmountOut args" This reverts commit 7991c1c. * change help text * add line to changelog (cherry picked from commit 3e26ec2) Co-authored-by: Richard Liu <[email protected]>
* Fix order for JoinSwapShareAmountOut CLI args (#3942) * fix order for JoinSwapShareAmountOut args * Revert "fix order for JoinSwapShareAmountOut args" This reverts commit 7991c1c. * change help text * add line to changelog (cherry picked from commit 3e26ec2) # Conflicts: # CHANGELOG.md * Update CHANGELOG.md * Update CHANGELOG.md Co-authored-by: Richard Liu <[email protected]> Co-authored-by: Roman <[email protected]> Co-authored-by: Dev Ojha <[email protected]>
Closes: #3813
What is the purpose of the change
Swaps the
token_in_max_amountandshare_out_amountCLI arguments of JoinSwapShareAmountOut to match spec in docs.Brief Changelog
token_in_max_amountandshare_out_amountin protoTesting and Verifying
Based off the example that @larry0x provided in the issue, I rebuilt osmosisd locally and ran the following to verify:
$ build/osmosisd tx gamm join-swap-share-amount-out uosmo 12345 67890 --pool-id 678 --from test1 --chain-id 1 {"body":{"messages":[{"@type":"/osmosis.gamm.v1beta1.MsgJoinSwapShareAmountOut","sender":"osmo1cyyzpxplxdzkeea7kwsydadg87357qnahakaks","pool_id":"678","token_in_denom":"uosmo","token_in_max_amount":"12345","share_out_amount":"67890"}],"memo":"","timeout_height":"0","extension_options":[],"non_critical_extension_options":[]},"auth_info":{"signer_infos":[],"fee":{"amount":[],"gas_limit":"350000","payer":"","granter":""}},"signatures":[]} confirm transaction before signing and broadcasting [y/N]: ^CAs seen above, the
token_in_max_amountbeing12345(the 2nd argument) andshare_out_amountbeing67890(the 3rd argument) indicate that the order of the arguments is now fixed.Documentation and Release Note
Unreleasedsection inCHANGELOG.md? no